-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8709 task: design tokens used to output styles and documentation (colours) #2
base: develop
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,39 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Work in progress, not currently used to output CSS or docs
* initial work with using design tokens and style dictionary to output css variables for styling and js modules for documentation
* initial work with using design tokens and style dictionary to output css variables for styling and js modules for documentation
1a6e6c6
to
c0e2ec6
Compare
src/stories/Colours.stories.tsx
Outdated
<tbody> | ||
{tokenKeys.map((key) => { | ||
const value = colourTokens[key]; | ||
const cssValue = /rgb/i.test(key) ? `rgb(${value})` : value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could fail if someone used the letters rgb
in their variable name. For example, FlagColourGB
. Doing a regex against the value might be safer than doing it against the key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line has been removed. Ultimately if we add RGB values we can handle that using a Style Dictionary transform.
src/tokens/colour/base.json
Outdated
"black": { "value": "#000" }, | ||
"black-rgb": { "value": "0, 0, 0" }, | ||
"white": { "value": "#fff" }, | ||
"white-rgb": { "value": "255, 255, 255" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering whether we couldn't auto-generate the rgb variants, rather than specify them manually ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that idea. We could also auto output different formats such as HSL in future. It would also negate the issues around key names you highlighted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RGB values removed from the token file in favour of automatically inserting using a Style Dictionary transform.
I've left a few comments. As a general feedback, I'm wondering whether generated files (the ones in Pros of including them in the repository:
Pros of generating them on-demand:
For me that last point - the risk of committing something inconsistent - is quite a strong one. It could be mitigated by CircleCi build checking for consistency. |
@aliceh75 thanks for the feedback. I agree with you that committing build files to the repo seems to be the wrong approach given the reasons you mentioned above. As I mentioned in the PR blurb the only reason they are included is because the CSS and documentation output depend on them but I'm still not 100% happy with the approach. If the build folder was ignored there is a risk of errors being raised when a user tries to build locally if the tokens haven't yet been generated. The obvious way to mitigate that would be to include "storybook": "npm run build:tokens && start-storybook -p 6006",
I guess that's the alternative. |
…ivalent for better robustness
… task/8709-design-token-prototyping # Conflicts: # .storybook/styles/storybook-app.scss
… task/8709-design-token-prototyping
* added token build process to build and dev processes to ensure generated variables are always available * removed rgb colour values from tokens list
10da041
to
d436a4c
Compare
… task/8709-design-token-prototyping
@aliceh75 further to this previous discussion, the npm script has been updated to ensure tokens are generated before dependent processes run. This is really a temporary stop-gap while we work out a more suitable architecture. |
…e naming to US English
Relates to wellcometrust/corporate#8709
Context
Design tokens will be the core of the design system long-term. Initially however the aim is to automate the process of outputting CSS variables for styling and also documentation for Storybook from a single source (i.e. a JSON file
src/tokens/colour/base.json
). Amazon's Style Dictionary enables us to do this very easily and for the long-term could be used to output other style formats e.g. iOS, Android, Kotlin.This PR is a proof of concept for flat variables. Further discovery needed to work out how to output CSS containing media queries
Design token build files (
src/build/css/colours.css
,src/build/css/colours.js
andsrc/build/css/colours.d.ts
) are included in this commit and not specifically ignored as they are dependencies for the CSS and Storybook output.To test
To run locally:
npm install
npm run storybook
and check Globals > Coloursnpm run build:tokens
to output css, js and d.ts tosrc/build
npm run build
to output React and CSS for consumption in thedist
folder